allow passing a callback to useFetch's run to modify init
#76
Conversation
phryneas
force-pushed
the
run-argument
branch
4 times, most recently
from
1b66fb5 to
ec58f61
Compare
|
Okay, now eslint should be happy, too. |
|
Okay, I've tried the failing |
|
Seems like the tests are generally breaking with react 16.9 and something |
|
I'm very happy with where you're going with this. I'm in doubt about the callback though. I like the flexibility it provides but I still would prefer to have the option to just pass an object. We can detect a SyntheticEvent using one of the attributes they have, such as My preference would be:
|
|
Sounds like a plan! I'll do that as soon as I'm back.
Am 10. August 2019 13:41:34 MESZ schrieb Gert Hengeveld <notifications@github.com>:
…I'm very happy with where you're going with this. I'm in doubt about
the callback though. I like the flexibility it provides but I still
would prefer to have the option to just pass an object. We can detect a
SyntheticEvent using one of the
[attributes](https://reactjs.org/docs/events.html#overview) they have,
such as `preventDefault`.
My preference would be:
- if function, use it to determine init
- if object and not SyntheticEvent, spread it after init
- else do nothing with init
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
|
phryneas
force-pushed
the
run-argument
branch
2 times, most recently
from
bae616c to
db67da9
Compare
|
Okay, that should do it for the functionality. I also pushed a commit that gives And I added an example. But unfortunately, I can't get the examples to run - are these examples currently runnable? It seems as if it can't parse the jsx tokens in |
phryneas
force-pushed
the
run-argument
branch
2 times, most recently
from
0e5ccae to
169c75a
Compare
|
I just rebased this on master and it seems to fail again - I guess this is due to some change on master? |
|
Yes the introduction of React v16.9 is causing some issues. On CircleCI things work fine but on Travis they fail. I suspect updating the examples to 16.9 might help. |
|
That seems to work. Should be fixed now. |
Codecov Report
@@ Coverage Diff @@
## master #76 +/- ##
==========================================
+ Coverage 98.91% 98.93% +0.01%
==========================================
Files 8 8
Lines 370 375 +5
Branches 136 140 +4
==========================================
+ Hits 366 371 +5
Misses 4 4
Continue to review full report at Codecov.
|
|
Okay, tests are now passing. |
ghengeveld
force-pushed
the
run-argument
branch
from
6340b08 to
cae8622
Compare
|
Released in v8.0.0 |
|
@all-contributors please add @phryneas for ideas |
|
I've put up a pull request to add @phryneas! 🎉 |
2 participants
Heyo :)
This adresses #75.
Instead of using a parameter to be spread to init, I chose for a callback function.
This has two reasons
runis just passed as an onClick handler, a React SyntheticEvent would have been passed and possibly spread on toinit. Detecting all kinds of events would be a pain, buttypeof x === "function"is a simple test that it is NOT an event.deep-mergeI also split up the method definition between
deferFnandpromiseFnbecause the handling of 2 parameters vs 3 parameters was already confusing enough (sometimes props is ctrl, sometimes ctrl is ctrl).I think this improves readability without impacting code size too much.
Also, I reduced boolean logic at two places to better handle the
undefinedcase ofdeferandjson.Of, and of course there's a new unit test and a bit more README :)
Looking forward to your feedback. 🙋